Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add official support for Java 21 #1940

Merged
merged 10 commits into from
Feb 3, 2025
Merged

Add official support for Java 21 #1940

merged 10 commits into from
Feb 3, 2025

Conversation

ecdye
Copy link
Contributor

@ecdye ecdye commented Jan 6, 2025

This also adds checks to make sure that Java 21 is installed before upgrading to openHAB 5.

Fixes #1937

@Larsen-Locke
Copy link
Contributor

zigbee2mqtt test is disabled here. Couldn't we enable it back?

@ecdye
Copy link
Contributor Author

ecdye commented Jan 12, 2025

zigbee2mqtt test is disabled here. Couldn't we enable it back?

Yep!, I just forgot to after we merged the fix. I'll do that now.

@ecdye ecdye force-pushed the jdk21 branch 3 times, most recently from 39618ef to bf8455e Compare January 13, 2025 06:18
@ecdye ecdye marked this pull request as ready for review January 13, 2025 06:32
@ecdye
Copy link
Contributor Author

ecdye commented Jan 13, 2025

@mstormi I can't quite fathom why, but OpenJDK 21 really doesn't like the GitHub Actions env. When I run it locally on Docker it works perfectly but not in Actions. I feel confident enough with all of this for it to be reviewed though.

I'm on the fence about defaulting to the Temurin over OpenJDK for 21 because of the issues I'm seeing here and because then it has to use unstable for the source. It also upgrades other packages when installing from unstable because they are dependencies which I don't really like (ie libc6). Temurin is also now the most widely supported JVM in general that Oracle doesn't build reference implementations anyways.

Let me know your thoughts.

(There ended up being a bunch of random fixes to automated testing that I found and added while debugging that stupid OpenJDK 21 issue that I never could figure out)

Signed-off-by: Ethan Dye <[email protected]>
Copy link
Contributor

@mstormi mstormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall although I'm unable to determine by just looking at the diffs if OpenJDK 21 is really available now and where it's coming from.

The firmware comments at the end are guessed but you probably wrote that code before you saw I revoked your recent changes.

tests/Dockerfile.amd64-installation Outdated Show resolved Hide resolved
tests/Dockerfile.rpi3-installation Outdated Show resolved Hide resolved
tests/Dockerfile.rpi5-installation Outdated Show resolved Hide resolved
@ecdye
Copy link
Contributor Author

ecdye commented Feb 3, 2025

LGTM overall although I'm unable to determine by just looking at the diffs if OpenJDK 21 is really available now and where it's coming from.

The firmware comments at the end are guessed but you probably wrote that code before you saw I revoked your recent changes.

Yeah I'll fix those. OpenJDK 21 is available from unstable and Temurin. Both are provided as options and should work well.

@ecdye ecdye requested a review from mstormi February 3, 2025 20:50
@mstormi
Copy link
Contributor

mstormi commented Feb 3, 2025

Alright. I think you should release a new image with this in, plus the fixed hotspot.
Don't forget to forward main to openHAB branch.

@ecdye ecdye merged commit 6f5beda into main Feb 3, 2025
10 checks passed
@ecdye ecdye deleted the jdk21 branch February 3, 2025 20:58
@ecdye
Copy link
Contributor Author

ecdye commented Feb 3, 2025

Should I make it another patch release or do a minor to 1.10? I'm thinking 1.10 because of the addition of Java 21 support.

WDYT?

@mstormi
Copy link
Contributor

mstormi commented Feb 3, 2025

yeah, 1.10. Went for beer (alcfree) and wanted to add that but you were faster :)

@mstormi
Copy link
Contributor

mstormi commented Feb 3, 2025

pls also add the changelog like in older releases, too.
(got word that the hotspot still doesn't do but it's bedtime for me today)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for Java 21 for OH5?
3 participants